Player Counter: Add Arma 3 and Arma Reforger support and improve Source query#115
Player Counter: Add Arma 3 and Arma Reforger support and improve Source query#115szpolny wants to merge 3 commits intopelican-dev:mainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds Arma 3 and Arma Reforger query type schemas, hardens the base SourceQueryTypeSchema with null-safe defaults and an early exit, updates plugin version to 1.1.0, and registers the new schemas in the plugin provider. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php`:
- Around line 41-51: Modify SourceQueryTypeSchema so its 'players' field uses
null as a sentinel when the query does not support a player list: call
$query->GetPlayers() and if it returns false/unsupported return null for
'players' instead of an empty array; update the array_map only when $players is
an array. This aligns SourceQueryTypeSchema with MinecraftBedrockQueryTypeSchema
and lets PlayerCounterController::players() (the is_null($data['players'])
check) behave as intended.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
player-counter/plugin.jsonplayer-counter/src/Extensions/Query/Schemas/Arma3QueryTypeSchema.phpplayer-counter/src/Extensions/Query/Schemas/ArmaReforgerQueryTypeSchema.phpplayer-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.phpplayer-counter/src/Providers/PlayerCounterPluginProvider.php
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (4)
player-counter/src/Extensions/Query/Schemas/ArmaReforgerQueryTypeSchema.php (2)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
SourceQueryTypeSchema(9-61)player-counter/src/Extensions/Query/Schemas/Arma3QueryTypeSchema.php (2)
getId(7-10)getName(12-15)
player-counter/src/Providers/PlayerCounterPluginProvider.php (3)
player-counter/src/Extensions/Query/Schemas/Arma3QueryTypeSchema.php (1)
Arma3QueryTypeSchema(5-16)player-counter/src/Extensions/Query/Schemas/ArmaReforgerQueryTypeSchema.php (1)
ArmaReforgerQueryTypeSchema(5-16)player-counter/src/Extensions/Query/QueryTypeService.php (1)
register(15-22)
player-counter/src/Extensions/Query/Schemas/Arma3QueryTypeSchema.php (2)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
SourceQueryTypeSchema(9-61)player-counter/src/Extensions/Query/Schemas/ArmaReforgerQueryTypeSchema.php (2)
getId(7-10)getName(12-15)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
player-counter/src/Http/Controllers/Api/Client/Servers/PlayerCounterController.php (2)
players(37-49)query(23-28)
🪛 GitHub Check: PHPStan (8.2)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php
[failure] 37-37:
Strict comparison using === between array and false will always evaluate to false.
🪛 GitHub Check: PHPStan (8.3)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php
[failure] 37-37:
Strict comparison using === between array and false will always evaluate to false.
🪛 GitHub Check: PHPStan (8.4)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php
[failure] 37-37:
Strict comparison using === between array and false will always evaluate to false.
🪛 GitHub Check: PHPStan (8.5)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php
[failure] 37-37:
Strict comparison using === between array and false will always evaluate to false.
🔇 Additional comments (6)
player-counter/plugin.json (1)
5-5: LGTM!Version bump to 1.1.0 appropriately reflects the addition of new features (Arma 3 and Arma Reforger support) following semantic versioning.
player-counter/src/Extensions/Query/Schemas/Arma3QueryTypeSchema.php (1)
5-16: LGTM!Clean and minimal implementation. Arma 3 uses the Steam/Source query protocol, so extending
SourceQueryTypeSchemais appropriate.player-counter/src/Providers/PlayerCounterPluginProvider.php (2)
10-11: LGTM!Imports are correctly added following the existing alphabetical ordering pattern.
40-41: LGTM!New query type schemas are properly registered following the established pattern.
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
37-39: The check is correct and necessary.The xPaw/PHP-Source-Query
GetInfo()method returnsbool|array— array on success, false on failure. The comparisonif ($info === false)is valid defensive code and should be retained.Likely an incorrect or invalid review comment.
player-counter/src/Extensions/Query/Schemas/ArmaReforgerQueryTypeSchema.php (1)
5-16: No changes required. Arma Reforger supports the standard Valve Source/Steam server query protocol (A2S), which is whatSourceQuery::SOURCEimplements. The implementation correctly inherits fromSourceQueryTypeSchemawithout custom engine handling, consistent with how Arma 3 is configured.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
41-51:⚠️ Potential issue | 🟠 MajorPreserve
nullfor unsupported player lists (not[]).Line 41 currently coerces
GetPlayers()failures/unsupported states into an empty array, which bypassesPlayerCounterController::players()null check and changes API semantics (unsupportedvsempty). This should stay distinguishable.Proposed fix
- $players = $query->GetPlayers() ?: []; + $players = $query->GetPlayers(); @@ - 'players' => array_map(fn ($player) => [ - 'id' => (string) ($player['Id'] ?? ''), - 'name' => (string) ($player['Name'] ?? 'Unknown'), - ], $players), + 'players' => is_array($players) + ? array_map(fn ($player) => [ + 'id' => (string) ($player['Id'] ?? ''), + 'name' => (string) ($player['Name'] ?? 'Unknown'), + ], $players) + : null,Also update the return phpdoc on Line 21 and Line 27 to allow
players: null|array<array{id: string, name: string}>.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php` around lines 41 - 51, The code currently forces $query->GetPlayers() into an empty array which hides "unsupported" results and breaks PlayerCounterController::players() null checks; change the handling in SourceQueryTypeSchema (where $players is assigned from $query->GetPlayers()) to keep null when GetPlayers() indicates unsupported (do not coerce to []), and update the method/class return phpdoc (the docblocks referenced around Line 21 and Line 27) to reflect 'players: null|array<array{id: string, name: string}>' so consumers know players may be null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php`:
- Around line 41-51: The code currently forces $query->GetPlayers() into an
empty array which hides "unsupported" results and breaks
PlayerCounterController::players() null checks; change the handling in
SourceQueryTypeSchema (where $players is assigned from $query->GetPlayers()) to
keep null when GetPlayers() indicates unsupported (do not coerce to []), and
update the method/class return phpdoc (the docblocks referenced around Line 21
and Line 27) to reflect 'players: null|array<array{id: string, name: string}>'
so consumers know players may be null.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php
📜 Review details
🧰 Additional context used
🧬 Code graph analysis (1)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (1)
player-counter/src/Http/Controllers/Api/Client/Servers/PlayerCounterController.php (2)
players(37-49)query(23-28)
🔇 Additional comments (2)
player-counter/src/Extensions/Query/Schemas/SourceQueryTypeSchema.php (2)
37-39: Good defensive early-return on missing query info.The guard on Line 37-39 prevents invalid data propagation and keeps failure handling explicit.
44-47: Null-safe typed defaults look solid.The fallback casts on Line 44-47 make the response shape stable under partial Source responses.
szpolny
force-pushed
the
player-counter/arma
branch
from
b0cfcb3 to
43f16be
Compare
|
Thanks but this isn't needed. You can simply use the |
2 participants
This pull request introduces support for Arma 3 and Arma Reforger server types to the Player Counter plugin, along with improvements for more robust server information querying and error handling.
New server type support
Arma3QueryTypeSchemaandArmaReforgerQueryTypeSchemaclasses to support querying Arma 3 and Arma Reforger servers. [1] [2]PlayerCounterPluginProviderso they are available for use. [1] [2]Plugin metadata
plugin.jsonto1.1.0to reflect the new features.Summary by CodeRabbit
New Features
Bug Fixes
Chores